Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: skip-subtrees option #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

akirak
Copy link
Contributor

@akirak akirak commented Jan 4, 2020

This implements an equivalent of org-agenda-todo-list-sublevels variable.

I basically tweaked the loop in org-ql--select as follows and added skip-subtrees argument to functions and macros in this package:

(cond (preamble (let ((case-fold-search preamble-case-fold))
                  (cl-loop while (re-search-forward preamble nil t)
                           do (outline-back-to-heading 'invisible-ok)
                           when (funcall predicate)
                           collect (funcall action)
                           when skip-subtrees
                           do (org-end-of-subtree)
                           do (outline-next-heading))))
      (t (cl-loop when (funcall predicate)
                  collect (funcall action)
                  when skip-subtrees
                  do (org-end-of-subtree)
                  while (outline-next-heading))))

It seems to be working, but I am not sure if I am doing the right thing in the preamble case. How should I add test cases?

@akirak akirak mentioned this pull request Jan 4, 2020
@alphapapa
Copy link
Owner

Hi Akira,

It looks like you've done a very thorough job here, and it should provide the feature you need. But I think the design is too specific to the feature, and I don't think we should add extra, feature-specific arguments and options if we can do it in a more flexible way.

Thinking about this idea again, I think a simpler solution might be something like this:

(org-ql-select files
  '(todo)
  :action '(prog1
               (org-element-headline-parser (line-end-position))
             (org-end-of-subtree)))

Looking at the loop in org-ql--select:

(cond (preamble (let ((case-fold-search preamble-case-fold))
                  (cl-loop while (re-search-forward preamble nil t)
                           do (outline-back-to-heading 'invisible-ok)
                           when (funcall predicate)
                           collect (funcall action)
                           do (outline-next-heading))))
      (t (cl-loop when (funcall predicate)
                  collect (funcall action)
                  while (outline-next-heading))))

The call to org-end-of-subtree in the action function puts point at the end of the subtree, before the next heading, and then outline-next-heading moves to the next heading to search for the next match. It should work both with and without preambles and without modifying any other code.

All that would remain would be to, possibly, wrap the pattern to make it easier to apply for the user, maybe something like:

(org-ql-select files
  '(todo)
  :action '(ql element skip-subtree))

Which would mean, roughly: "The action is an org-ql special action form (which implies developing an "action" vocabulary and documenting it, of course, but that opens up a number of possibilities in a flexible way). First, return the element (same as the element action), then skip the subtree."

Some other ideas for the action forms:

(org-ql-select files
  '(todo)
  ;; Intending to emphasize that `skip-subtree' is not what's returned.
  :action '(ql element :then skip-subtree))
(org-ql-select files
  '(todo)
  ;; Using existing Lisp forms.
  :action '(ql (prog1 element (skip-subtree)))

Or, for this case at least, maybe we wouldn't need a special action form, but just a few special functions like skip-subtree, which would just be a shorter alias for org-end-of-subtree, like:

(org-ql-select files
  '(todo)
  ;; Using existing Lisp forms.
  :action '(prog1 element (skip-subtree))

What do you think?

Thanks for your work on this. Regardless of the code that we finally use, you've helped move us toward the final solution.

@akirak
Copy link
Contributor Author

akirak commented Jan 4, 2020

Thank you for your comment.

It looks like you've done a very thorough job here.

Thanks to counsel-rg and noccur, it was easy for me to find corresponding argument lists. I looked for occurrences of narrow and added skip-subtree to each list. It was not as a tough job as it might seem.

But I think the design is too specific to the feature, and I don't think we should add extra, feature-specific arguments and options if we can do it in a more flexible way.

You are right. We want to keep the API as minimal as possible while extending its functionality.

(org-ql-select files
  '(todo)
  :action '(prog1
               (org-element-headline-parser (line-end-position))
             (org-end-of-subtree)))

I haven't thought of this idea, and it would work for org-ql-select and its alternatives. I'm always impressed with your skills in writing Emacs Lisp. And any of your suggested syntaxes look good.

My question is, though, how about agenda views, i.e. org-ql-view and org-ql-block? They don't have :action argument, so we won't be able to directly augment the action to jump to the end of each matching subtree. If we added :skip-subtrees argument and supported extra options in org-ql-block as you suggested in #79, org-ql-block would support the feature in a consistent API. How would you support this feature in org-ql-view and org-ql-block?

@alphapapa
Copy link
Owner

That's a good point. I think we can add support for the :action argument in org-ql-view:

(list (-let* (((&plist :buffers-files :query :sort :narrow :super-groups :title) view)
(that's not the only line where it needs to be added). The default will remain the same, passing :action nil if none is specified. And if we add keyword argument support to org-ql-block as discussed, I think that will handle that as well. What do you think?

@akirak
Copy link
Contributor Author

akirak commented Jan 4, 2020

And if we add keyword argument support to org-ql-block as discussed, I think that will handle that as well. What do you think?

That will be good. Please feel free to close this PR and work on the feature.

Back to the original discussion, I don't mind whichever style you choose. I now prefer the following form due to its balance between conciseness and explicitness:

  :action '(prog1 element (skip-subtree))

I personally even don't need the skip-subtree shorthand. prog1 already states that the result of the second expression is discarded, and so skip-subtree can be considered just an alias for org-end-of-subtree, which saves only several characters. However, for those who are not used to Org mode scripting, org-end-of-subtree might not sound familiar, so the shorthand can be useful. We will need a consistent explanation for how element is substituted in this case, though.

I also thought of (element :skip-subtree t) or (nil :skip-subtree t) where the action is considered a special form when the first argument is a specific keyword. I have no idea which is the best.

@alphapapa
Copy link
Owner

Thanks. I'll plan to work on this after tagging 0.4, and I'll leave this open for now since it has useful discussion about the plans.

@alphapapa alphapapa force-pushed the master branch 6 times, most recently from 492ed77 to c4f9a58 Compare January 10, 2020 16:36
@alphapapa alphapapa force-pushed the master branch 2 times, most recently from 1359cbb to b3601cf Compare January 19, 2020 05:32
@alphapapa alphapapa force-pushed the master branch 2 times, most recently from af2582f to 8baeeea Compare February 9, 2020 21:25
@alphapapa
Copy link
Owner

alphapapa commented Mar 9, 2020

I just had another idea about how to set the subtree-skipping option: an optional property list at the beginning of a query. Something like:

(org-ql-select files
  '(and :skip-subtrees t (todo))
  :action 'element)

I don't love that idea, but it is somewhat idiomatic Elisp, e.g. like define-minor-mode's optional keyword arguments before its BODY forms. And it would make it possible to set when writing the query form, rather than having to set another argument separately, or having to modify the action.

@akirak
Copy link
Contributor Author

akirak commented Mar 10, 2020

That syntax looks strange at first but an excellent idea at the same time. It's probably the best way to support the feature discussed in this thread, especially in that it would allow dynamically enabling the feature in helm-org-ql. Also, it fits org-ql-views without changing the type definition. I am willing to use the syntax once it is available.

Still, and :skip-subtrees t is a bit too long to type frequently in helm-org-ql, so I will probably define an abbreviation (#97) for the arguments.

@alphapapa
Copy link
Owner

Yeah, it does seem strange and familiar at the same time. Maybe we should try to get more opinions on the syntax. :)

@alphapapa alphapapa force-pushed the master branch 2 times, most recently from e441d88 to 4194456 Compare November 9, 2020 02:07
@alphapapa alphapapa force-pushed the master branch 3 times, most recently from f52bef0 to ec0f8c7 Compare November 16, 2020 11:23
@alphapapa alphapapa modified the milestones: 0.5, 0.6 Nov 20, 2020
@alphapapa
Copy link
Owner

I don't love that idea, but it is somewhat idiomatic Elisp, e.g. like define-minor-mode's optional keyword arguments before its BODY forms. And it would make it possible to set when writing the query form, rather than having to set another argument separately, or having to modify the action.

Looking at that idea again, I really don't like it, because it would mean that queries would no longer be predicate sexps. Maybe a wrapper form could be used, something like (skipping-subtrees (todo)), and the query pre-processor could "unwrap" it and set an option or something accordingly.

@akirak
Copy link
Contributor Author

akirak commented Nov 20, 2020

Or maybe

(org-ql-select files
  '(todo)
  :action '(no-descendants element))

Because of what it actually does, it may be better to allow the preprocessor in the action.

@akirak
Copy link
Contributor Author

akirak commented Nov 20, 2020

Looking at that idea again, I really don't like it, because it would mean that queries would no longer be predicate sexps.

That makes sense. The feature probably don't deserve the added complexity, so I am welcome to the rejection.

@alphapapa
Copy link
Owner

alphapapa commented Nov 20, 2020

Looking at that idea again, I really don't like it, because it would mean that queries would no longer be predicate sexps.

That makes sense. The feature probably don't deserve the added complexity, so I am welcome to the rejection.

To be clear, what I meant that I don't like is this syntax I suggested:

(org-ql-select files
  '(and :skip-subtrees t (todo))
  :action 'element)

I'm still interested in a way to skip subtrees in queries.

Or maybe

(org-ql-select files
  '(todo)
  :action '(no-descendants element))

Because of what it actually does, it may be better to allow the preprocessor in the action.

Yeah, I think this needs some more thought and design. Thanks. :)

@alphapapa alphapapa force-pushed the master branch 3 times, most recently from ae83de1 to 890c247 Compare November 24, 2020 09:59
@akirak
Copy link
Contributor Author

akirak commented Nov 26, 2020

To be clear, what I meant that I don't like is this syntax I suggested

I'm still interested in a way to skip subtrees in queries.

Yes, I know what you mean.

Thanks to your hard work, org-ql has got custom predicates. Perhaps there would be a generalized way to define custom actions or action transformers.

@alphapapa
Copy link
Owner

That's an interesting idea. Thanks.

@alphapapa alphapapa modified the milestones: 0.6, Future Jun 17, 2021
@akirak
Copy link
Contributor Author

akirak commented Oct 30, 2021

Rather than extending org-ql-select or the query language to support subtree skipping, how about supporting the feature in org-ql-search and dynamic blocks for now?

Because those interfaces don't accept the action argument, the user cannot implement subtree skipping right now, unless he/she writes a complex query like (and QUERY (not (ancestors QUERY))). This currently limits use cases of the package, and I actually want subtree skipping in one of my dynamic blocks now, where a complex query like above would be a mess. (Or is it possible to support custom meta queries which transform the query inside itself?)

I think it is safer to add an argument to dynamic blocks than to extend the query language or org-ql-select. Once you extend any of them to support subtree skipping, you probably can refactor org-ql-search and dynamic blocks to use the new syntax/argument, with ease. The users of org-ql-select can already implement subtree skipping on their own, so changing the lower APIs can be considered later.

@yantar92
Copy link
Contributor

yantar92 commented Oct 30, 2021 via email

@akirak
Copy link
Contributor Author

akirak commented Oct 30, 2021

@yantar92 I didn't look into org-defpred well. Your code example apparently doesn't work, but using org-defpred seems to be a solution. Thank you for your information.

@yantar92
Copy link
Contributor

yantar92 commented Oct 30, 2021 via email

@akirak
Copy link
Contributor Author

akirak commented Oct 30, 2021

@yantar92 No, it doesn't work. It doesn't produce what I expect.

Instead, the following worked:

(org-ql-defpred skip-subtrees (query)
  "Run QUERY skipping the whole subtree when it fails."
  :normalizers
  ((`(root ,query)
    `(and ,query
          (not (ancestors ,query))))))

This may be inefficient, so it would be better if there were a solution that uses org-end-of-subtree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants